Fixes, improvements #109
Fixes, improvements #109
Conversation
Handle block devices exposed by VMs
Replaced by BlockDevice extension
qubesd isn't running at this stage yet.
Install both stubdom implementations: mini-os one (xen-hvm) and linux one (xen-hvm-stubdom-linux). QubesOS/qubes-issues#2185
While libvirt handle locking itself, there is also Qubes-specific
startup part. Especially starting qrexec-daemon and waiting until
qrexec-agent connect to it. When someone will attempt to start VM the
second time (or simply assume it's already running) - qrexec will not be
connected yet and the operation will fail. Solve the problem by wrapping
the whole vm.start() function with a lock, including a check if VM is
running and waiting for qrexec.
Also, do not throw exception if VM is already running.
This way, after a call to vm.start(), VM will be started with qrexec
connected - regardless of who really started it.
Note that, it will not solve the situation when someone check if VM is
running manually, like:
if not vm.is_running():
yield from vm.start()
Such code should be changed to simply:
yield from vm.start()
Fixes QubesOS/qubes-issues#2001
Fixes QubesOS/qubes-issues#2666
It was related to DispVM savefile preparation, but it is no longer applicable in Qubes 4.0
Since we have qrexec-based updates proxy, we can even stronger isolate templates from outside threats. QubesOS/qubes-issues#1854
vm.kernel property have type 'str'. Putting None there makes a lot of troubles: it gets encoded as 'None' in qubes.xml and then loaded back as 'None' string, not None value. Also it isn't possible to assign None value to str property throgh Admin API. kernel='' is equally good to specify "no kernel from dom0". QubesOS/qubes-issues#2622
Make them call into qubesd. Create separate socket for "misc" calls - VM accessible, but not part of Admin API.
Move inheriting volume from template to a helper function. No functional change. QubesOS/qubes-issues#2256
Re-init volume config of all 'snap_on_start' volumes at template chanage. For this, save original volume config and re-use config_volume_from_source function introduced in previous commit. At the same time, forbid changing template of running AppVM or any DispVM. QubesOS/qubes-issues#2256
Again, if libvirt or even Xen isn't running, we can safely assume VM isn't too.
Specifically, check if root volume is updated after template switch.
dom0 isn't real VM and most properties doesn't apply to it. Lets make it more explicit.
With libvirt in place, this isn't enough - libvirt also keep VM configuration in its memory and adjusting xenstore doesn't change that. In fact changing xenstore behind it back make it even worse in some situations. QubesOS/qubes-issues#1426
Codecov Report
@@ Coverage Diff @@
## master #109 +/- ##
=========================================
+ Coverage 42.04% 43.24% +1.2%
=========================================
Files 45 47 +2
Lines 8025 8266 +241
=========================================
+ Hits 3374 3575 +201
- Misses 4651 4691 +40
Continue to review full report at Codecov.
|
|
This also cover QubesOS/qubes-issues#1858 |
|
|
||
| @qubes.api.method('qubes.NotifyTools', no_payload=True) | ||
| @asyncio.coroutine | ||
| def qubes_notify_tools(self): |
There was a problem hiding this comment.
This method should also fire 'features-request' event and the following logic should be in an extension. The content of untrusted_features should be of course collected from /qubes-tools/. That way it will be possible to update those tools in this distribution to use new API.
qubes/api/misc.py
Outdated
|
|
||
| @qubes.api.method('qubes.NotifyUpdates') | ||
| @asyncio.coroutine | ||
| def qubes_notify_updates(self, untrusted_payload): |
There was a problem hiding this comment.
I think 'updates_available' could be a feature, not a property. This is more of a policy than a mechanism.
| @@ -170,10 +172,10 @@ def send_exception(self, exc): | |||
| self.transport.write(str(exc).encode('utf-8') + b'\0') | |||
|
|
|||
|
|
|||
| def sighandler(loop, signame, server, server_internal): | |||
| def sighandler(loop, signame, *servers): | |||
There was a problem hiding this comment.
This one will conflict with woju/qubes-core-admin@65a7326
| :param payload: payload of the request | ||
| :param args: request to qubesd | ||
| :return: | ||
| ''' |
There was a problem hiding this comment.
You created a docstring. This is very commendable. 👍
qubes/vm/qubesvm.py
Outdated
| if value is None: | ||
| return value | ||
| if value is None or value == '': | ||
| return '' |
BaseVM have no (useful) __str__ method.
Make qubes.NotifyTools reuse logic of qubes.FeaturesRequest, then move actual request processing to 'features-request' event handler. At the same time implement handling 'qrexec' and 'gui' features request - allowing to set template features when wasn't already there. Behavior change: template is no longer allowed to change feature value (regardless of being True or False). This means the user will always be able to override what template have set.
| if 'version' in untrusted_features: | ||
| # any suspicious string will raise exception here, | ||
| # but otherwise ignored | ||
| int(untrusted_features['version']) |
There was a problem hiding this comment.
This looks wrong. First of all, it's pointless: if we are doing nothing (short of raising exception), maybe we shouldn't be parsing this at all. Second, why this wasn't renamed to "qubes-tools/version" or something like that? Remember this is received by all extensions, not just this one.
There was a problem hiding this comment.
This is version of qubes tools (or rather: dom-VM interface of it). Currently we don't have any need for negotiating this, but we may need in the future. So, having (Windows) VM exposing "1" here may be useful in the future.
There was a problem hiding this comment.
There is nothing that prevents VM from exposing "1" or whatever it pleases without parsing this on dom0's end. When the need arises, someone will be rewriting the extension anyway, so we can just drop this now. Until then, we just ignore unknown keys, this one included.
As I said, I think it's not just useless, it actually sends unqualified "version" feature request to all extensions, without saying what exactly is versioned (gui? qrexec? the qubesdb handshake?).
This is because event handlers of features-request don't generally get context of the call and I'd imagine they contain something like:
if 'my_expected_feature' in untrusted_features:
# ...From extension writer's perspective, there are some problems hiding around this handler. For one, we never promised that one qubes.FeaturesRequest will arrive as single event (and we obviously won't make such promise).
OTOH, other features' names (esp. "gui" and "qrexec") are good precisely because they are very general and I think more than one extension will have interest in them.
It isn't used for anything, so simply ignore it for good. QubesOS#109 (comment)
A bunch of mostly unrelated commits - see individual commit messages.
WARNING: fa1da42 change qubes.xml format - old file will not load. You need to manually remove dom0 attributes (name, qid, uuid).